To complete the context, this is a separate concern that @wtdcode has expressed to me in a DM right after #3367 (comment) having worked on the very same functionality in https://github.com/bluealloy/revm/blob/6d5256eaa9212e17da5a9fd8e9131fc85a0e7ad9/crates/revm/src/db/ethersdb.rs#L35-L63.

In particular, the current implementation of this function relies on the fact that handle.runtime_flavor() will not return RuntimeFlavor::CurrentThread to work correctly. Currently this point is ensured in two places, but the link here is implicit, making it potentially difficult to find for future maintainers:

This PR tries to document more caveats on this point and cause run_future() to panic on a wrong runtime setup. For more info on this, see tokio-rs/tokio#4563.

Hmmm -- IMO we should just remove all uses of run_future() instead. Not sure why we still need it?

Hmmm -- IMO we should just remove all uses of run_future() instead. Not sure why we still need it?

@djc Yes, that's the end goal for us, but looking at the removal of deprecated() in e92b85e or the recently-restarted transition to rustls (#3790) I guess it's still better to be a bit more defensive :)

Hmmm -- IMO we should just remove all uses of run_future() instead. Not sure why we still need it?

@djc Yes, that's the end goal for us, but looking at the removal of deprecated() in e92b85e or the recently-restarted transition to rustls (#3790) I guess it's still better to be a bit more defensive :)

Not sure how/why those are related to this issue?

Hmmm -- IMO we should just remove all uses of run_future() instead. Not sure why we still need it?

@djc Yes, that's the end goal for us, but looking at the removal of deprecated() in e92b85e or the recently-restarted transition to rustls (#3790) I guess it's still better to be a bit more defensive :)

Not sure how/why those are related to this issue?

@djc Those are two examples of a transient state ended up remaining much longer than originally expected (x_x), I mean it would be perfect if this time it ends sooner, but still I think this PR would be a nice addition for the moment being even if the lines it adds end up being deleted in months.

Hmmm -- IMO we should just remove all uses of run_future() instead. Not sure why we still need it?

@djc Yes, that's the end goal for us, but looking at the removal of deprecated() in e92b85e or the recently-restarted transition to rustls (#3790) I guess it's still better to be a bit more defensive :)

Not sure how/why those are related to this issue?

@djc Those are two examples of a transient state ended up remaining much longer than originally expected (x_x), I mean it would be perfect if this time it ends sooner, but still I think this PR would be a nice addition for the moment being even if it ends up being deleted in months.

To ask the question differently: why do you think we need to keep utils::run_future() around today?

To ask the question differently: why do you think we need to keep utils::run_future() around today?

@djc I think we can agree that this is a temporary scaffolding function before we make the full async transition, as previously explained pretty clearly in #3367 (comment):

If that is changed naively by just making main() async, and reqwest async, then we get rust's async function tainting everything and will change all the relevant calls to async, but there are a number of places where types need to change to be compatible with async requirements; so the commit becomes huge and unwieldy very quickly.

So the approach I'm taking is some scaffolding that lets us work this incrementally, and once complete it will just be nice async code.

Apart from the signature generalization (which you might argue as some form of premature optimization), this PR is actually mostly documenting this function's limitations (multithread runtime only, and a limit of 512 for blocking threads), so that one might get some clue about a potential misuse.

I was not saying that I wish the transient state would last long... Actually I wish it could end today. But maybe I'm being overly cautious?

I think run_future is very close to gone right? I didn't actually intend to leave it in the migration to an async runtime.

Closing in favor of #3856. Thanks anyway @wtdcode!